Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Build OpenJ9 Windows jdk19+ with VS2022, and disable ccache #3251

Merged
merged 2 commits into from
May 18, 2023
Merged

Build OpenJ9 Windows jdk19+ with VS2022, and disable ccache #3251

merged 2 commits into from
May 18, 2023

Conversation

pshipton
Copy link
Contributor

@pshipton pshipton commented Feb 14, 2023 •
edited
Loading

OpenJ9 was missing the change to disable ccache for all versions.

Issue eclipse-openj9/openj9#16701

@github-actions github-actions bot added the windows Issues that affect or relate to the WINDOWS OS label Feb 14, 2023
@pshipton pshipton mentioned this pull request Feb 14, 2023
Merged
@pshipton
Copy link
Contributor Author

@AdamBrousseau fyi

@github-actions github-actions bot added the openj9 Issues that are enhancements or bugs raised against the OpenJ9 group label Feb 14, 2023
@pshipton pshipton marked this pull request as ready for review February 14, 2023 22:04
@pshipton pshipton changed the title Build OpenJ9 Windows jdk19+ with VS2022 Build OpenJ9 Windows jdk19+ with VS2022, and disable ccache Feb 14, 2023
karianna
karianna requested changes Feb 15, 2023
View reviewed changes
Copy link
Contributor

@karianna karianna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In /github/workspace/build-farm/platform-specific-configurations/windows.sh line 227:
fi
^-- SC1089 (error): Parsing stopped here. Is this keyword correctly matched up?

@pshipton
Build OpenJ9 Windows jdk19+ with VS2022, and disable ccache
c213222 
OpenJ9 was missing the change to disable ccache for all versions.

Issue eclipse-openj9/openj9#16701

Signed-off-by: Peter Shipton <Peter_Shipton@ca.ibm.com>
@pshipton
Copy link
Contributor Author

Oops, fixed now.

@karianna
Copy link
Contributor

OK the Alpine build is failing because it is merging/generating a file that has a merge conflict: <<<<<<< HEAD

No idea where this is coming from as a GitHub code search indicates that's nowhere in the entire Adoptium org.

@pshipton pshipton mentioned this pull request Feb 16, 2023
Merged
sxa
sxa reviewed Feb 22, 2023
View reviewed changes
Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does --disable-ccache do anything on Windows JDK builds? I didn't think it was possible to use it there.

@pshipton
Copy link
Contributor Author

Does --disable-ccache do anything on Windows JDK builds? I didn't think it was possible to use it there.

I don't know, I'm just making it consistent. 32-bit builds use it, non-openj9 builds use it, and openj9 jdk8 64-bit uses it. My change uses it for openj9 64-bit builds of all versions.

@sxa
Copy link
Member

sxa commented Mar 13, 2023

@karianna Are you happy with this now after Peter addressed your request?

@karianna
Copy link
Contributor

OK the Alpine build is failing because it is merging/generating a file that has a merge conflict: <<<<<<< HEAD

No idea where this is coming from as a GitHub code search indicates that's nowhere in the entire Adoptium org.

The Alpine build still seems to have a merge conflict?

@gdams
Copy link
Member

gdams commented Mar 15, 2023

I think we should try and align hotspot/openj9 better here. @sxa I think we were talking about bumping the Temurin builds to VS 2022 as well?

github-actions[bot]
github-actions bot previously requested changes Mar 15, 2023
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A block has been put on this Pull Request as this repository is temporarily under a code freeze due to an ongoing release cycle.

If this pull request needs to be merged during the release cycle then please comment /merge and a PMC member will be able to remove the block.

If the code freeze is over you can remove this block by commenting /thaw.

@sxa
Copy link
Member

sxa commented Mar 15, 2023

OK the Alpine build is failing because it is merging/generating a file that has a merge conflict: <<<<<<< HEAD
No idea where this is coming from as a GitHub code search indicates that's nowhere in the entire Adoptium org.

The Alpine build still seems to have a merge conflict?

That's the separate branch that MSFT is maintaining I believe - @gdams can you check this one?

@sxa
Copy link
Member

sxa commented Mar 15, 2023

I think we should try and align hotspot/openj9 better here. @sxa I think we were talking about bumping the Temurin builds to VS 2022 as well?

Possibly but we can do that under a separate PR if desired. At the moment all the work that is being done to enable reproducibility for Temurin is on VS2019 and additional work would be required to move it up.

@AdamBrousseau
Copy link
Contributor

Can this be merged?

@karianna
Copy link
Contributor

I think we're still in release freeze.

@sxa
Copy link
Member

sxa commented May 17, 2023

/thaw

@github-actions github-actions bot dismissed their stale review May 17, 2023 14:53

Pull Request unblocked - code freeze is over.

@karianna karianna merged commit a971b70 into adoptium:master May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gdams gdams gdams approved these changes

@sxa sxa sxa approved these changes

@AdamBrousseau AdamBrousseau AdamBrousseau approved these changes

@karianna karianna karianna approved these changes

@github-actions github-actions[bot] github-actions[bot] left review comments

Assignees
No one assigned
Labels
openj9 Issues that are enhancements or bugs raised against the OpenJ9 group windows Issues that affect or relate to the WINDOWS OS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pshipton @karianna @sxa @gdams @AdamBrousseau